Skip to content

Conversation

felixrabe
Copy link
Contributor

@felixrabe felixrabe commented Aug 1, 2018

I can still reproduce #39364 with the example code at #39364 (comment).

I'm opening this PR in an attempt to document this bug as a known issue in libstd/sync/mpsc/mod.rs.

Inputs very much welcome. (Nightly docs for recv_timeout.)

@oberien
Copy link
Contributor

oberien commented Aug 1, 2018

I think the documentation should have a small explanation of the bug, so that people can see on a quick glance if they might be affected. Moreover I think some sort of explanation why it hasn't been fixed yet should be added, otherwise it might seem to outsider like "Oh look at Rust, they know they have a critical bug in one of the core synchronization primitives and don't fix it. How should I ever be able to trust that language!!1!1".

@felixrabe felixrabe changed the title Document #39364 Document #39364 – Panic in Receiver::recv_timeout Aug 1, 2018
@felixrabe felixrabe changed the title Document #39364 – Panic in Receiver::recv_timeout Document #39364 – Panic in mpsc::Receiver::recv_timeout Aug 1, 2018
@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 6, 2018
@kennytm
Copy link
Member

kennytm commented Aug 6, 2018

Thanks for the PR! We’ll periodically check in on it to make sure that @alexcrichton or someone else from the team reviews it soon.

@alexcrichton
Copy link
Member

Thanks! Could this be expanded a bit to say that this is an unexpected panic which is a bug in the standard library? Otherwise the current documentation sort of seems like it's expected to be buggy...

@felixrabe
Copy link
Contributor Author

Ok, I've added the reproducible example for people to see if they are affected, mentioning that it is a "currently ... unexpected" panic.

As to explaining the bug or why it isn't fixed yet, I'm at a loss.

@felixrabe
Copy link
Contributor Author

Btw I don't have docs set up, so I can't test whether the comments render fine.

@rust-highfive

This comment has been minimized.

@@ -1249,7 +1249,29 @@ impl<T> Receiver<T> {
///
/// # Panics
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this section should be Bugs or Known problems rather than Panics?

/// use std::sync::mpsc::channel;
/// use std::thread;
/// use std::time::Duration;
///
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[00:03:46] tidy error: /checkout/src/libstd/sync/mpsc/mod.rs:1259: trailing whitespace

@TimNN
Copy link
Contributor

TimNN commented Aug 14, 2018

Ping from triage @alexcrichton: This PR requires your review.

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Collaborator

bors commented Aug 14, 2018

📌 Commit 025f41f has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 14, 2018
@bors
Copy link
Collaborator

bors commented Aug 14, 2018

⌛ Testing commit 025f41f with merge 5bb2094...

bors added a commit that referenced this pull request Aug 14, 2018
Document #39364 – Panic in mpsc::Receiver::recv_timeout

I can still reproduce #39364 with the example code at #39364 (comment).

I'm opening this PR in an attempt to document this bug as a known issue in [libstd/sync/mpsc/mod.rs](https://github.com/rust-lang/rust/blob/master/src/libstd/sync/mpsc/mod.rs).

Inputs very much welcome. ([Nightly docs for `recv_timeout`.](https://doc.rust-lang.org/nightly/std/sync/mpsc/struct.Receiver.html?search=#method.recv_timeout))
@bors
Copy link
Collaborator

bors commented Aug 15, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 5bb2094 to master...

@bors bors merged commit 025f41f into rust-lang:master Aug 15, 2018
@felixrabe felixrabe deleted the patch-1 branch April 23, 2019 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants